-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix for No VMs start after Renew Host Security Keys due to wrong qemu group reading #11632
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.20 #11632 +/- ##
=========================================
Coverage 16.17% 16.17%
- Complexity 13296 13299 +3
=========================================
Files 5656 5656
Lines 498144 498151 +7
Branches 60437 60441 +4
=========================================
+ Hits 80584 80587 +3
- Misses 408590 408592 +2
- Partials 8970 8972 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@StepBee |
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Sure @weizhouapache , i tested the following cases |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14991 |
|
@blueorangutan test |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
@blueorangutan test ubuntu24 kvm-ubuntu24 |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ubuntu24 mgmt + kvm-ubuntu24) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-14327)
|
|
[SF] Trillian test result (tid-14328)
|
weizhouapache
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
verified OK with different settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a bug in the Host Security Keys renewal process where QEMU group name parsing from the libvirtd configuration file was failing due to improper handling of spaces and quotes. The fix ensures VMs can start properly after Host Security Keys renewal.
- Updated QEMU group name extraction logic to handle spaces and quotes in configuration values
- Fixed regex pattern to match group configuration lines with optional whitespace
- Improved AWK parsing to correctly extract the group name value
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
vishesh92
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. tested different variations in the conf and qemu group is getting extracted correctly after the changes.
|
@StepBee however, @Copilot suggested to use the following , which looks more easy also, do we consider single quote in the value, for example |
I agree with @weizhouapache. IMO we should consider single quotes as well in the configuration. |
Simplify and correct qemu group extraction from configfile Co-authored-by: Wei Zhou <[email protected]>
Thank you @vishesh92 @weizhouapache i agree, the awk was not required and it's good to catch single quote as well during this fix. |
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
weizhouapache
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
retest the commands with more values, looks good
thanks @StepBee for the update
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15004 |
|
@blueorangutan test |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
@blueorangutan test ubuntu24 kvm-ubuntu24 |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ubuntu24 mgmt + kvm-ubuntu24) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-14342) |
|
@blueorangutan test |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
Merging based on approvals and manual tests |
|
Awesome work, congrats on your first merged pull request! |
|
[SF] Trillian test result (tid-14343)
|
|
[SF] Trillian test result (tid-14344)
|
Description
This PR fixes reading the libvirt/qemu group name from the libvirtd qemu config file including spaces and quotes,
which is used during the deployment of Host Security Keys.
This PR fixes #11144
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Tested renewal of Host Security Keys on Ubuntu 24.04 hosts and stopping and starting VMs afterwards.
How did you try to break this feature and the system with this change?